Skip to content

fix pinecone transform sub-key - #1720

Open
erichanwang wants to merge 2 commits into
rocketride-org:developfrom
erichanwang:fix/RR-1664-pinecone-sub-key
Open

fix pinecone transform sub-key#1720
erichanwang wants to merge 2 commits into
rocketride-org:developfrom
erichanwang:fix/RR-1664-pinecone-sub-key

Conversation

@erichanwang

@erichanwang erichanwang commented Jul 29, 2026

Copy link
Copy Markdown

fixes #1664

changes

  • key Pinecone transform state by hash(api key), profile, and index
  • keep raw API key material out of transform tags
  • document the one-time re-ingest behavior after the key changes, using the accurate legacy host/port/collection wording from CodeRabbit review

CodeRabbit

  • addressed the README migration-note wording thread by replacing the misleading index-only legacy description with legacy host/port/collection key

verification

  • PYTHONPATH=nodes/src:nodes/test:packages/server/engine-lib/rocketlib-python/lib /home/ericwang/Projects/upstream-contributions/rocketride-issue1638/.venv/bin/python -m pytest nodes/test/store_pinecone/test_transform_sub_key.py -q -> 2 passed
  • /home/ericwang/Projects/upstream-contributions/rocketride-issue1638/.venv/bin/python -m ruff check nodes/src/nodes/store_pinecone/IGlobal.py nodes/test/store_pinecone/test_transform_sub_key.py
  • /home/ericwang/Projects/upstream-contributions/rocketride-issue1638/.venv/bin/python -m ruff format --check nodes/src/nodes/store_pinecone/IGlobal.py nodes/test/store_pinecone/test_transform_sub_key.py
  • git diff --check
  • pre-commit hook: gitleaks

Summary by CodeRabbit

  • Bug Fixes
    • Improved Pinecone transform key generation to avoid exposing API keys and to use a more secure, consistent derived key format.
    • Transform keys are now reliably different across API keys and configurations, with suffixes reflecting the selected profile and collection.
  • Documentation
    • Clarified that changing the API key, profile, or collection updates the transform key, and existing pipelines may require a one-time re-ingestion after upgrading.
  • Tests
    • Added automated tests covering transform key derivation and ensuring API keys are never present in the generated key.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 212eb2f2-b380-4992-8a65-396f42f8c045

📥 Commits

Reviewing files that changed from the base of the PR and between 083f835 and 963d81a.

📒 Files selected for processing (1)
  • nodes/src/nodes/store_pinecone/README.md

📝 Walkthrough

Walkthrough

Pinecone transform sub-keys now combine a truncated SHA-256 API-key hash with profile and collection. Documentation covers transform-key changes and possible one-time re-ingestion, while tests validate destination separation and credential exclusion.

Changes

Pinecone transform key

Layer / File(s) Summary
Update Pinecone sub-key derivation
nodes/src/nodes/store_pinecone/IGlobal.py, nodes/src/nodes/store_pinecone/README.md
IGlobal._sub_key uses a truncated API-key hash, profile, and collection; authentication documentation describes transform-key changes and possible re-ingestion.
Validate sub-key isolation
nodes/test/store_pinecone/test_transform_sub_key.py
Tests verify distinct API keys produce distinct sub-keys without exposing credentials, and that profile and collection appear in the suffix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jmaionchi, stepmikhaylov, rod-christensen

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main Pinecone sub-key fix.
Linked Issues check ✅ Passed The new hashed API-key/profile/collection sub-key matches the issue and the README documents the one-time re-ingestion impact.
Out of Scope Changes check ✅ Passed The code, test, and README changes all support the Pinecone sub-key fix and migration note.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added docs Documentation module:nodes Python pipeline nodes labels Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor
🤖 Internal: Discord sync marker

Auto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete.

@erichanwang
erichanwang marked this pull request as ready for review July 30, 2026 20:14
@erichanwang

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nodes/src/nodes/store_pinecone/README.md`:
- Line 102: Update the migration note in the Pinecone README to accurately
describe the legacy transform-key inputs as host, port, and collection, or
replace the misleading index-only wording with a neutral reference to older
transform-key formats. Keep the documented re-ingestion behavior unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 30187a5e-635f-4b46-b032-77591e2c5903

📥 Commits

Reviewing files that changed from the base of the PR and between 2cbd567 and d2bce83.

📒 Files selected for processing (3)
  • nodes/src/nodes/store_pinecone/IGlobal.py
  • nodes/src/nodes/store_pinecone/README.md
  • nodes/test/store_pinecone/test_transform_sub_key.py

Comment thread nodes/src/nodes/store_pinecone/README.md Outdated
@erichanwang
erichanwang force-pushed the fix/RR-1664-pinecone-sub-key branch from d2bce83 to 083f835 Compare July 30, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation module:nodes Python pipeline nodes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

store_pinecone: transform sub-key does not distinguish destinations

1 participant